Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/superpowers/plans/2026-05-22-session-bound-permission-dialogs.md`:
- Around line 159-167: useEffect 中当前直接 return unsubscribe 导致不是一个 cleanup
function；把订阅的返回值（unsubscribe）包成一个函数并在该函数中调用其 dispose() 方法。定位到使用
useEffect、permissionBridgeService.onActiveSessionChange、unsubscribe、permissionBridgeService.getActiveSession
和 setActiveSessionId 的代码，把 return unsubscribe; 改为 return () =>
unsubscribe.dispose();（若 onActiveSessionChange 返回的是一个函数而非对象则改为 return () =>
unsubscribe();）。

In `@docs/superpowers/plans/2026-05-26-acp-webmcp-groups.md`:
- Around line 518-523: The parsing logic incorrectly expects parts[0] to be an
empty string which rejects valid methods like "_opensumi/file/read"; update the
conditional around method, parts, groupName and toolAction so it explicitly
checks that parts.length === 3, parts[0] === '_opensumi' (the prefix), and
parts[1] is non-empty, and only then assign groupName = parts[1] and toolAction
= parts[2]; adjust the error branch to return TOOL_NOT_FOUND when these explicit
checks fail.

In `@docs/superpowers/specs/2026-05-26-acp-webmcp-groups-design.md`:
- Around line 60-63: The doc's described behavior (dynamic registration of
`_opensumi/{group}/*` on load_group/unload_group and JSON-RPC -32601 when not
loaded) is inconsistent with the current implementation which routes all calls
through extMethod and returns a business error TOOL_NOT_LOADED; update either
the docs or the implementation to match: either change the docs in this spec to
describe the extMethod routing and TOOL_NOT_LOADED response (referencing
load_group/unload_group and ACP Server notification semantics), or modify the
implementation so ACP Server actually registers/unregisters methods at
load_group/unload_group and returns standard JSON-RPC Method not found (-32601)
when a group method is not loaded; ensure you reference extMethod,
TOOL_NOT_LOADED, load_group, unload_group and ACP Server in the chosen fix so
SDK/tests follow the correct contract.

In `@packages/ai-native/__test__/node/acp/acp-thread.test.ts`:
- Around line 23-30: The Jest mock for stream/web doesn't match the actual
import used in acp-thread.ts (which imports from 'node:stream/web'), so update
the mock target in acp-thread.test.ts to jest.mock('node:stream/web', ...) and
ensure it exports MockReadableStream and MockWritableStream matching the
original mock's shape; locate the existing MockReadableStream/MockWritableStream
definitions in the test and replace the module specifier string only so the mock
applies to the import used by the code under test.
- Around line 272-290: The test currently installs its own 'exit' handler and
directly mutates private fields, which bypasses AcpThread's real exit handling;
instead assign the mock child process to (thread as any)._childProcess (using
createMockChildProcess), do not add an 'exit' listener in the test, then emit
'exit' on that mock so AcpThread's internal handler (which updates
_processRunning, _connected and status) runs; assert (thread as
any)._processRunning is false, (thread as any)._connected is false and
thread.status is 'disconnected'. Use the same symbols: _childProcess,
_processRunning, _connected and status on the AcpThread instance so the test
verifies the class's real logic.

In `@packages/ai-native/__test__/node/permission-routing.test.ts`:
- Around line 60-88: The tests contain missing awaits and empty assertions: in
the 'should register a session' case await the async call to
service.routePermissionRequest(baseRequest, 'sess-1') and assert
mockCallerService.requestPermission was called with (baseRequest, 'sess-1') and
optionally assert the returned outcome equals the mocked response; in 'should
unregister a session' await service.routePermissionRequest(baseRequest,
'sess-1') after unregisterSession('sess-1') and assert the result is the
cancelled outcome (or that mockCallerService.requestPermission was not called)
using expect(...).toEqual(...) or toHaveBeenCalledTimes(0); in 'should not
affect other sessions when unregistering one' await
service.routePermissionRequest(baseRequest, 'sess-2') and assert
mockCallerService.requestPermission was called with (baseRequest, 'sess-2') to
prove sess-2 remains routable. Ensure all routePermissionRequest calls are
awaited and assertions use mockCallerService.requestPermission and returned
values.

In `@packages/ai-native/package.json`:
- Around line 63-64: The package currently allows zod v3 and v4 but the MCP
proxy (packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts) only
calls tool.inputSchema.toJSONSchema() when present, so running with Zod v3
leaves a non-JSON Schema passed to ListTools; either tighten package.json "zod"
to "^4.0.0" or add a v3 fallback: import zod-to-json-schema and, inside the code
path that reads tool.inputSchema (in mcp-server-proxy.service.ts), when
inputSchema.toJSONSchema is undefined call zodToJsonSchema(inputSchema) and
ensure the returned value is a valid JSON Schema before sending to MCP; also
review packages/ai-native/src/common/tool-invocation-registry.ts for any
Zod-4-only usage (e.g., z.record(z.string(), ...)) and adjust or guard
imports/types if you choose to keep v3 support.

In `@packages/ai-native/src/browser/acp/permission-bridge.service.ts`:
- Around line 244-279: In clearSessionDialogs, the requestId prefix check is
unsafe; use the reverse map requestIdToSessionId to find which requestIds belong
to the session instead of comparing requestId.startsWith(prefix). Iterate
requestIdToSessionId to collect all requestIds where sid === sessionId, then for
each id: remove from activeDialogs, remove from pendingDecisions (clear timeout,
resolve with {type: 'cancelled'} and fire onPermissionResult), delete from
requestIdToSessionId, and delete from pendingBySessionId as currently done;
ensure onPendingCountChangeEmitter.fire() is called if any pending/session
entries were removed to preserve the existing notifications.

In `@packages/ai-native/src/browser/acp/webmcp-file-tools.registry.ts`:
- Around line 51-53: resolveWorkspacePath currently just joins workspaceDir and
relativePath, allowing path traversal (e.g., "../") to escape the workspace and
affect file_read/file_write/file_delete; update resolveWorkspacePath to compute
absolute paths (use path.resolve) for both workspaceDir and the joined target,
then verify the target is inside the workspace by checking
path.relative(workspaceAbs, targetAbs) does not start with ".." (or that
targetAbs starts with workspaceAbs + path.sep) and throw an error if it escapes;
return the validated absolute (or normalized) target path so callers like
file_read/file_write/file_delete are protected.

In `@packages/ai-native/src/browser/acp/webmcp-group-registry.ts`:
- Around line 57-76: The executeTool method currently lets errors from
tool.execute bubble up, breaking the expected WebMcpToolResult contract; wrap
the call to tool.execute in an async try/catch (await the promise) inside
executeTool and on any thrown error return a structured failure WebMcpToolResult
(e.g., success: false, error: 'TOOL_EXECUTION_ERROR' or similar, and details
containing the error message/stack) rather than propagating the exception;
update references inside executeTool, notably the local variables group, method,
tool and the final return value, so all execution paths return a
WebMcpToolResult.

In `@packages/ai-native/src/browser/acp/webmcp-groups/file.webmcp-group.ts`:
- Around line 21-26: The resolveWorkspacePath function currently concatenates
paths and returns them without normalization or boundary checks, enabling path
traversal (e.g., "../" or absolute paths) to escape the workspace; update
resolveWorkspacePath to: normalize and resolve the combined path using a proper
path resolver (e.g., path.resolve/path.posix.resolve) against workspaceDir,
canonicalize any symlinks if applicable, then verify that the resolved absolute
path starts with the canonicalized workspaceDir (or throw/return an error) and
only return the resolved path if it is inside the workspace; reference
resolveWorkspacePath to locate and replace the current logic.

In `@packages/ai-native/src/browser/acp/webmcp-tools.registry.ts`:
- Around line 516-546: The option parsing in execute is too permissive: the
current kind derivation uses args.optionId.includes('allow') which can
misclassify arbitrary strings as allowed; update execute to strictly validate
args.optionId against the allowed enum values ('allow_once', 'allow_always',
'reject'), map each explicitly to the kind passed to
permissionBridge.handleUserDecision, and return an INVALID_INPUT error for any
other value; locate the logic around execute, args.optionId, the kind variable
and the call to AcpPermissionBridgeService.handleUserDecision (retrieved via
tryGetService) and replace the includes-based branch with an explicit switch/if
that enforces the whitelist and error path.

In `@packages/ai-native/src/browser/acp/webmcp-utils.ts`:
- Around line 51-53: The current check in webmcp-utils.ts that returns
'DI_ERROR' uses msg.includes('di') which causes false positives; update the
conditional that examines the input variable msg (the branch that returns
'DI_ERROR') to match only explicit DI-related tokens or phrases (e.g., whole
word "di" via word-boundary regex, "dependency injection",
"dependency-injection", "di container", "DIContainer",
"DependencyInjectionError", "injector") instead of a simple substring; keep the
existing injector check but replace msg.includes('di') with a stricter pattern
or set of explicit keyword checks so non-DI messages like "invalid input" are
not misclassified.

In `@packages/ai-native/src/browser/chat/acp-chat-agent.ts`:
- Around line 111-115: The log in getRequestOptions currently prints sensitive
data (API key fragments and full config); change logging to avoid any secret
exposure by only logging non-sensitive booleans/flags (e.g., whether apiKey is
present, whether baseURL is set, model/modelId names if allowed) instead of
apiKey.slice or the full config; update the logger calls in getRequestOptions
and the similar block around lines 169-183 to replace API key/config outputs
with something like "apiKeyConfigured=true/false" and avoid printing partial
keys or full config objects (refer to the getRequestOptions method and the other
logger calls in acp-chat-agent.ts).

In `@packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts`:
- Around line 31-44: The current mapping in mcp-server-proxy.service.ts bypasses
TypeScript checks with "as any" and may return a Zod v3 schema object (not valid
JSON Schema) when .toJSONSchema is absent; update the code in the block that
builds tools so that you detect whether tool.inputSchema has a toJSONSchema
method (e.g., using "typeof (tool.inputSchema as unknown)?.toJSONSchema ===
'function'") and if not, import and call zodToJsonSchema from the
zod-to-json-schema package to convert Zod v3 schemas, or alternatively enforce
Zod v4 in package.json and remove the fallback; ensure you replace the "as any"
cast with a proper type-narrowing check and return a valid JSON Schema in
inputSchema for each tool.

---

Minor comments:
In @.claude/skills/cdp-verification-scenarios/SKILL.md:
- Around line 90-95: The fenced code blocks containing the checklist lines
("Observe actual state (via CDP or WebMCP)" through "Output explicit judgment:
PASS or FAIL") and the other blocks at the indicated ranges are missing a
language tag, causing MD040; update each triple-backtick fence to include a
language token such as ```text or ```md (e.g., change ``` to ```text) for the
blocks that contain the checklist and the blocks at ranges 130-144 and 150-168
so the linter stops flagging them.

In @.claude/skills/dev-loop/SKILL.md:
- Around line 25-29: Add a language identifier to the fenced code block that
contains the flow diagram (the block starting with "Phase 0: 环境准备..."), e.g.
change the opening fence from ``` to ```text so the block becomes ```text ...
```, which will satisfy Markdown lint rule MD040.

In `@docs/superpowers/plans/2026-05-20-acp-node-sdk-refactor.md`:
- Around line 21-57: The markdown has many lint warnings (MD040/MD028) because
fenced code blocks lack language identifiers and blockquotes contain extra blank
lines; update all fenced code blocks in this document (including the sections
showing AcpThread, AcpAgentService, AcpPermissionRpcService, AcpCliBackService,
AcpFileSystemHandler, AcpTerminalHandler diagrams and the long plan sections
referenced) to include an appropriate language token (e.g., text, typescript,
bash) and remove any blank lines inside blockquote sections so blockquotes are
continuous; ensure you apply the same fixes to the other ranges called out
(lines 63-180, 184-237, 241-1240) to eliminate MD040/MD028 warnings.

In `@docs/superpowers/plans/2026-05-25-dev-loop-skill-implementation.md`:
- Around line 231-547: Several fenced code blocks in the dev-loop plan (the
large Markdown content in
docs/superpowers/plans/2026-05-25-dev-loop-skill-implementation.md) lack
language identifiers; add appropriate language tags to each triple-backtick
fence (e.g., markdown for Markdown snippets, bash for shell commands, javascript
for CDP/WebMCP snippets, json for configuration) so blocks such as the
Scenario/Phase examples, the shell/git commands, the JSON config, and the CDP
evaluate_script example are explicitly annotated and avoid markdownlint
warnings.

In `@docs/superpowers/specs/2026-05-21-acp-thread-full-delegation-design.md`:
- Around line 33-60: 为两个示意代码块补上语言标识以修复 MD040：在包含接口/类示意图的代码块（显示 IAcpAgentService
和 AcpThread 的那一段）以及文件中另一个相同类型的代码块（评论中提到的 66-68 行附近），把开头的 ``` 改为
```text（或使用更贴近的语言标识），确保示意图的代码块标记为 text 以消除 MD040 警告并保持渲染一致；定位时参考符号
IAcpAgentService 和 AcpThread 以找到对应段落并修改代码块标记。

In `@docs/superpowers/specs/2026-05-22-acp-webmcp-testing-example.md`:
- Around line 45-47: In the markdown file
2026-05-22-acp-webmcp-testing-example.md, many fenced code blocks are missing
language identifiers (triggering MD040); update every triple-backtick block to
include an explicit language tag (e.g., ```text, ```json, ```tsx, ```mermaid)
matching the block content—for example the snippet "Agent 通过 Chrome DevTools MCP
连接到打开的 IDE 页面 (http://localhost:8080)" should be wrapped as ```text; search the
file for all fenced blocks (the ones called out in the review) and add the
appropriate language specifier so linting and rendering are consistent.

In
`@docs/superpowers/specs/2026-05-22-session-bound-permission-dialogs-design.md`:
- Around line 32-52: The fenced code blocks in this document (showing the old
and new flow) lack a language identifier and trigger markdownlint MD040; update
both blocks to use a language tag (for example ```text) so the blocks containing
the flow diagrams (references: the code blocks under the "Node: AcpThread ..."
diagrams) include a language marker and resolve CI/markdownlint failures; ensure
both occurrences around the "Old Flow" and "New Flow" diagrams use the same tag.

In `@docs/superpowers/specs/2026-05-22-webmcp-tool-granularity.md`:
- Around line 31-190: Several fenced code blocks in this spec (e.g., the
examples showing startFileCreation vs createFile, the return-semantic examples,
the usage snippets for searchFiles/getEditorState/runDiagnostics, the
anti-pattern code blocks like navigator.modelContext.registerTool examples, and
the ASCII flowchart) lack language tags and trigger markdownlint MD040; update
each triple-backtick fence to include an appropriate language hint (use
javascript for JS snippets, text for prose/ASCII diagrams, or json when showing
JSON-like returns) so tools and linters correctly highlight and MD040 is
resolved.

In `@docs/superpowers/specs/2026-05-22-webmcp-tool-registration-design.md`:
- Around line 22-325: The doc has many fenced code blocks without language
identifiers which trips the linter; update each triple-backtick block in this
file (including the ASCII diagram, the TypeScript example for
registerAcpWebMCPTools, the flow snippets, tables and shell snippets) to include
an appropriate language tag such as text, typescript, or bash (e.g., replace ```
with ```text or ```typescript / ```bash as appropriate) so examples like the
webmcp-tools.registry.ts snippet and the codegraph_explore/flow blocks are
lint-clean.

In `@docs/superpowers/specs/2026-05-25-dev-loop-design.md`:
- Around line 245-267: The tree snippet claims webmcp-examples.md is moved to
dev-loop/reference/ but the PR intent is to delete it and replace its content
with webmcp-tool-registrar/CODE-PATTERNS.md; update the spec so the entry for
dev-loop/reference/ does not list webmcp-examples.md and instead documents that
webmcp-examples.md is removed and its guidance is superseded by
webmcp-tool-registrar/CODE-PATTERNS.md (referencing webmcp-examples.md,
dev-loop/reference, and webmcp-tool-registrar/CODE-PATTERNS.md to locate the
relevant files).

In
`@docs/superpowers/specs/2026-05-26-background-permission-notification-design.md`:
- Around line 49-82: The fenced code block in the spec lacks a language
identifier which triggers MD040; update the block opening from ``` to include a
language (e.g., ```text or a more specific language) so the markdown linter is
happy—apply this change to the block that documents the Node layer and Browser
layer (containing symbols like AcpThread.handlePermissionRequest,
AcpPermissionCallerService.requestPermission, AcpPermissionBridgeService, and
methods
showPermissionDialog/handleUserDecision/getPendingCountExcludingActive/hasPendingForSession).

In `@packages/ai-native/__test__/browser/permission-dialog-ui.test.tsx`:
- Around line 104-105: The file-level single instance mockPermissionBridge
created via createMockPermissionBridgeService() is reused across tests causing
listener state to leak; remove the top-level instantiation and instead recreate
and inject a fresh mock by calling createMockPermissionBridgeService() inside a
beforeEach, then use that per-test mock when mounting components or wiring
services (reference the mockPermissionBridge identifier and
createMockPermissionBridgeService() factory and update any setups that currently
rely on the shared instance); ensure any afterEach cleanup detaches listeners if
applicable so each test starts with a clean mock.

In `@packages/ai-native/__test__/browser/webmcp-tools.test.ts`:
- Around line 135-141: The test currently asserts
navigator.modelContext!.getTools().length === 12 which is brittle; instead call
navigator.modelContext!.getTools(), filter the result for tools whose name
starts with 'acp_' (e.g. tool.name.match(/^acp_\w+$/) or startsWith('acp_')),
then assert the filtered array length is 12 and loop over the filtered tools to
assert they lack an 'execute' property and their names match the expected
pattern; update assertions to use the filtered list (references: getTools,
navigator.modelContext, tool.name).

In `@packages/ai-native/__test__/node/acp-thread-status-caller.test.ts`:
- Around line 64-68: The test currently uses a synchronous check
(expect(...).not.toThrow()) which won't catch Promise rejections; change the
assertion to await the async call and assert it resolves (e.g., await
expect(service.notifyThreadStatusChange('session-1',
'working')).resolves.toBeUndefined()) so the mocked rejection from
mockRpcClient.$onThreadStatusChange is properly asserted as swallowed by
notifyThreadStatusChange.

In `@packages/ai-native/src/browser/acp/components/AcpChatHistory.tsx`:
- Line 199: Remove the stray direct rendering of the boolean value
item.hasPendingPermission in the AcpChatHistory component (it causes "true" to
appear in the DOM); locate the JSX that contains "{item.hasPendingPermission}"
and either delete that expression or replace it with a proper conditional
UI/indicator (e.g., render an icon or text only when item.hasPendingPermission
is true using a conditional render) so the boolean itself is not output to the
DOM.

In `@packages/ai-native/src/browser/acp/webmcp-groups/terminal.webmcp-group.ts`:
- Around line 60-84: The schema exposes a name field but it is never used when
creating the terminal; update the execute logic to pass params.name into
terminalController.createTerminal (e.g., include a displayName or name property
in the options object) so the declared name becomes effective, or if you prefer
not to support naming remove the name entry from the schema/description to avoid
misleading callers; locate the execute function where
terminalController.createTerminal is called and either add the name propagation
(params.name as string) to the createTerminal options or remove the name schema
entry.

---

Nitpick comments:
In `@docs/superpowers/plans/2026-05-22-session-bound-permission-dialogs.md`:
- Around line 79-87: The example Promise stores a pending decision with a bogus
cast `undefined as unknown as NodeJS.Timeout`; change the shape so
`pendingDecisions` entries make `timeout` optional (e.g., timeout?:
NodeJS.Timeout) or omit `timeout` entirely in the "no-auto-timeout" path instead
of asserting types; update the code that reads or clears
`pendingDecisions.get(requestId)?.timeout` to handle the optional field and any
callers of the structure (look for uses of pendingDecisions, requestId, and
timeout in resolve/clear logic) so no type-unsafe assertions remain.

In `@packages/ai-native/src/browser/acp/build-agent-process-config.ts`:
- Around line 8-35: The current buildAcpAgentProcessConfig uses
input.userPreferences.nodePath || undefined which collapses an explicit empty
string to undefined; update the assignment for nodePath in
buildAcpAgentProcessConfig to preserve an explicit empty string (i.e. only
convert to undefined when nodePath is strictly undefined) by replacing the
truthy-coercion with an explicit check against undefined, and adjust tests or
add a new test to cover the distinct behavior between "" and undefined for
userPreferences.nodePath.

In `@packages/ai-native/src/browser/acp/components/AcpChatHistory.tsx`:
- Around line 215-220: Remove the commented-out debug span block inside the
AcpChatHistory component: delete the entire commented JSX that contains
data-testid={`thread-status-${item.id}`} and references item.threadStatus and
item.loading so the file no longer contains commented debugging code; after
removal, run a quick lint/format pass to ensure the JSX remains well-formed.